Skip to content

Conversation

@FND
Copy link
Contributor

@FND FND commented Jul 7, 2018

this supersedes #39

test_browserslist
transpiling JavaScript for Chrome 63
transpiling JavaScript for IE 11
`dist/bundle.js`: 0 kB β†’ 0 kB (Ξ” 38 %)
`dist/bundle_alt.js`: 23 kB β†’ 0 kB (Ξ” 98 %)
`dist/bundle_legacy.js`: 23 kB β†’ 0 kB (Ξ” 98 %)
⚠️ this bundle looks to be fairly big - you might want to double-check whether that's intended and consider performance implications for your users: `dist/bundle.js`
βœ“ dist/bundle.js
βœ“ dist/bundle_alt.js
βœ“ dist/bundle_legacy.js

format: "iife"
},
reporting: {
threshold: 100000 // ~100 kB
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure this is actually correct: 1000 vs. 1024 πŸ€·β€β™‚οΈ

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Speaking of which, I've used 1000 in rollup-plugin-analyzer for formatting sizes. Do you think that might ought to be a configurable option there? Sorry to drop in here to ask, was just looking over this and noticed that might be something I should make configurable as well.

Copy link
Contributor Author

@FND FND Jul 11, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We appreciate you dropping by.

I've just tried this:

let fs = require("fs");
let txt = fs.readFileSync("/tmp/lipsum.txt");
Buffer.byteLength(txt, "utf8") / 1000; // 282.292
Buffer.byteLength(txt, "utf8") / 1024; // 275.676

The file system (both on Debian and macOS) reports 276 kB, so I suppose 1024 is correct - which would also mean it doesn't need to be configurable?

// NB: bundle-specific configuration can override global options
bundleConfig = Object.assign({ compact }, bundleConfig);
return makeBundle(bundleConfig, assetManager.resolvePath, { browsers });
return makeBundle(bundleConfig, assetManager, { browsers });
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not happy with the increasing amount of context we have to pass through in situations like this - not sure that can be avoided though. (In this case, we need assertManager.referenceDir so we can pass it to rollup-plugin-analyzer, eventually, via Bundle β†’ generateBundle; it all seems a little too convoluted.)

report: ({ size, originalSize, reduction }) => {
let b2kb = i => `${Math.round(i / 1024)} kB`;
console.error(`${bundle}: ${b2kb(originalSize)} β†’ ` +
`${b2kb(size)} (Ξ” ${Math.round(reduction)} %)`);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd consider this a placeholder for now, until we've figured out detailed reporting (see above). It sort of duplicates AssetManager#writeFile's reporting.

@moonglum
Copy link
Member

moonglum commented Jul 8, 2018

Interesting work, need to look at it in detail πŸ‘ A unified reporting concept for CSS, JS and images would be a great feature for faucet 1.1

caveats:
* there might be performance implications:
  doesdev/rollup-plugin-analyzer#3
* no detailed report yet:
  doesdev/rollup-plugin-analyzer#2
* missing configuration options (cf. inline comment)
@FND FND force-pushed the bundle-survey branch from e715941 to fe90c0b Compare July 11, 2018 06:02
@FND FND force-pushed the master branch 4 times, most recently from 3e9a3cd to a4b1278 Compare January 8, 2020 06:48
@moonglum moonglum changed the base branch from master to main March 8, 2022 07:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants